-
Notifications
You must be signed in to change notification settings - Fork 2.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Support C-Style enum
in preproc
#1984
Conversation
Converting to draft because I kind of forget the |
5609af3
to
e8291f3
Compare
e8291f3
to
aef6170
Compare
fixed the issue and rebased. |
my 2ct that pretty much applies to all of preproc is that papering over things with a custom preprocessor that parses "just enough" of the syntax but not all of it will cause a lot of confusion and frustration from developers down the line, who will expect the compiler to behave like a normal C compiler/assembler. Take for example the following code:
I don't think things like this will work with the current PR as-is, and I can't begin to think what other syntax quirks may pop up when people try doing more out-there things. I should mention that the snippet above primarily wouldn't work because for some reason, assembler files are first passed through |
Agreed, but I'd wager the lack of any support for C-style enums in assembly files is a bigger headache than incomplete support. We're already mixing C header files into assembly, so the door for this kind of confusion is open. These sorts of incompatibilities are basically always going to be a problem with preproc like you said, and replacing it is beyond this PR. |
The C preprocessor has its own syntax that can be clearly discerned by the Anyway, I'm not asking to remove preproc as a whole, but make you consider that this feature might make the situation worse by making certain problems harder to debug: If the issue I pointed out in the previous message is not accointed for it's rather easy to accidentally make an enum that has different values depending on whether it's compiled for ASM or for C. I'd like to try to minimize this sort of thing, at the very least... |
I can very confidently say that there are more users that will be served by this PR versus users that are expecting the C compiler to behave a certain way. The majority of our userbase doesn't even know what a compiler is! |
If newbies are modifying C, often with the intention of learning C, it's especially destructive to introduce hard-to-debug footguns like this. How do you expect newbies to realize that the code they learned from google, a friend, or a book, isn't valid here? That's what I'm concerned about: This preprocessor isn't complete enough to be C, and it's not obvious that it's not C. People will expect it to be C and be confused when their game breaks. |
mid-kid is right that the preprocessor interacts weirdly with our assembler pipeline. But there's already some sharp edges in that interaction: I can't remember exactly the examples, but I think people get themselves into a mess using So I think as long as the SBird, I can take a look at PRing these changes to your branch if you'd like? |
a81181d
to
cdda673
Compare
Incorporated MGriffins changes to address the following issues:
E: I generally agree that |
This likewise fails to build for me, with only the usage information as output. I suspect it's a difference with |
Gives basic support for parsing C-Style
enum
expressions when usingpreproc
.Description
In order to deal with situations where one would rather want to use an
enum
but can't becausepreproc
does not know how to deal with it when compiling scripts and other assembly files together with regular C TUs.Example:
test.h
:test.s
:turns into
GNU-Style line indicators are added s.t. errors can be traced back to the original source file (e.g. if an
enum
field is supposed to be redefined)Discord contact info
karathan